Fix addMainModuleInfo erroneously considered UP-TO-DATE
#17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The addMainModuleInfo task replaces the original JAR with a modifies JAR. However, previously it did not properly specify the task inputs, therefore it was erroneously considered UP-TO-DATE even if the JAR changed. Note that now unfortunately the `jar` task is always run and never considered UP-TO-DATE because addMainModuleInfo modified the original JAR.
Marcono1234
commented
Aug 23, 2022
| abstract String getShortName() | ||
| @InputFile | ||
| abstract File getInputJar() |
There was a problem hiding this comment.
Have not specified @PathSensitive(PathSensitivity.NONE) here because for AddMainModuleInfoTask the path matters since the file is overwritten.
I hope this does not cause issues for ModuleConfiguration which overrides this method and adds its own annotations.
| @@ -100,7 +100,6 @@ class ModuleConfiguration extends AbstractModuleConfiguration { | |||
| primaryArtifact.file | |||
| } | |||
|
|
|||
| @Input | |||
There was a problem hiding this comment.
I assume this is redundant because the overridden method already defines this (but have not tested this!).
Comment on lines
+23
to
+25
| @Input | ||
| String group | ||
| @Input |
There was a problem hiding this comment.
I think it makes sense to define these as inputs, but I have not tested this.
| abstract String getVersion() | ||
| @Nested |
There was a problem hiding this comment.
Have not tested this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
addMainModuleInfotask replaces the original JAR with a modifies JAR. However, previously it did not properly specify the task inputs, therefore it was erroneously considered UP-TO-DATE even if the JAR changed.Note that now unfortunately the
jartask is always run and never considered UP-TO-DATE becauseaddMainModuleInfomodified the original JAR. Not sure if that can be solved in a cleaner way. Though interestingly manually specifying the JAR in thebuild.gradle.ktswithinputs.file(mainModule.get().inputJar)does not cause thejartask to be re-run (though maybe that also depends on the Gradle version being used).I am not that familiar with Gradle so any feedback is appreciated!
Note that there are some parts which I have not tested; if you want I can omit those and only add the annotations necessary for
getInputJar().